Implement graceful shutdown in Fluffy #2645
Conversation
| @@ -50,7 +50,7 @@ type | |||
| GetBoolCallback* = proc(): bool {.gcsafe, raises: [].} | |||
| GetSlotCallback* = proc(): Slot {.gcsafe, raises: [].} | |||
|
|
|||
| LightClientManager* = object | |||
| LightClientManager* = ref object | |||
There was a problem hiding this comment.
Had to change this to a ref object so that the stop proc can become async
There was a problem hiding this comment.
And making LightClientManager in the stop call a var did not help?
There was a problem hiding this comment.
I tried that and it didn't work. This was the error: Error: 'self' is of type <var LightClientManager> which cannot be captured as it would violate memory safety, declared here: /home/user/development/status-im/nimbus-eth1/fluffy/network/beacon/beacon_light_client_manager.nim(323, 12); using '-d:nimNoLentIterators' helps in some cases. Consider using a <ref var LightClientManager> which can be captured.
| @@ -45,7 +45,7 @@ proc start*(n: BeaconNode) = | |||
| n.beaconNetwork.start() | |||
|
|
|||
| proc stop*(n: BeaconNode) {.async.} = | |||
| n.beaconNetwork.stop() | |||
| discard n.beaconNetwork.stop() | |||
There was a problem hiding this comment.
I think these can just await also now?
There was a problem hiding this comment.
I tried that but it slows down and even breaks the tests in some cases. Probably because the tests stop and start the network multiple times. Adding the discard effectively allows the tests to behave as before.
fluffy/portal_node.nim
Outdated
| metricsServer*: Opt[MetricsHttpServerRef] | ||
| rpcHttpServer*: Opt[RpcHttpServer] | ||
| rpcWsServer*: Opt[RpcWebSocketServer] |
There was a problem hiding this comment.
I'm not a big fan of adding these to the PortalNode object.
Conceptually, to be a part of the Portal network as a node you don't require these more user faced features.
PortalNode is also there to be able to easily integrate our portal node in for example nimbus-eth1 binary. And while these are optional, you still end up importing rpcserver and chronos_httpserver.
If we want to make this more streamlined then I would rather create another object called PortalClient that holds all these servers + PortalNode + interacts with anything cli config related.
There was a problem hiding this comment.
I see your point. Yeah I wasn't sure about this part either. Okay, I'll move them out and clean them up separately for now. I'll keep the PortalClient idea for a separate PR.
No description provided.